refactor: seperate user from session #5403
Conversation
# Conflicts: # server/src/api/boards.go # server/src/api/router.go # server/src/sessions/service.go
mateo-ivc
force-pushed
the
mi/seperate-user-from-session
branch
from
990b0ef to
207eb3a
Compare
|
The deployment to the dev cluster was successful. You can find the deployment here: https://5403.development.scrumlr.fra.ics.inovex.io Deployed Images
|
server/src/sessions/database.go
Outdated
| ) | ||
|
|
||
| type DB struct { | ||
| type SessionDB struct { |
There was a problem hiding this comment.
Rename to DB to be consistent
server/src/sessions/service.go
Outdated
| database UserDatabase | ||
| sessionService SessionService | ||
| realtime *realtime.Broker | ||
| type BoardSessionService struct { |
There was a problem hiding this comment.
Rename to Service to be consistent
| const dtoToParticipant = async (dto: ParticipantDTO): Promise<Participant> => { | ||
| const user: Auth = await API.getUserById(dto.id); | ||
| return { | ||
| user, | ||
| connected: dto.connected, | ||
| raisedHand: dto.raisedHand, | ||
| ready: dto.ready, | ||
| showHiddenColumns: dto.showHiddenColumns, | ||
| role: dto.role, | ||
| banned: dto.banned, | ||
| }; | ||
| }; | ||
|
|
||
| const mapParticipantsWithUsers = async (message: BoardInitEvent): Promise<Participant[]> => { | ||
| const {participants} = message.data; | ||
| const asDTOs = participants as unknown as ParticipantDTO[]; | ||
| const mapped = await Promise.all(asDTOs.map(dtoToParticipant)); | ||
| message.data.participants = mapped; | ||
| return mapped; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Why don't we use batch fetching here instead of querying every user one after the other? With 10 participants, that's 10 separate API calls that could potentially be just one. What do you think about
the performance impact when boards get busy?
Similar pattern in the websocket handlers around line 178-186
I see we're doing the same sequential fetching in multiple places. Have you considered if there's a way we could consolidate this logic?
There was a problem hiding this comment.
If we going over to a batch API we could do something like this, to fix both, the performance and possible issues when participants aren't loading.
const mapParticipantsOptimized = async (dtos: ParticipantDTO[]): Promise<Participant[]> => {
try {
const userIds = dtos.map(dto => dto.id);
const users = await API.getUsersByIds(userIds);
return dtos.map(dto => {
const user = users.find((u: Auth) => u.id === dto.id);
if (!user) {
console.warn(`User not found for participant ${dto.id}`);
return null;
}
return { user, ...dto };
}).filter(Boolean) as Participant[];
} catch (error) {
console.error('Failed to map participants with users:', error);
return [];
}
};
| } | ||
| }, | ||
|
|
||
| getUserById: async (userId: string) => { |
There was a problem hiding this comment.
What happens if getUserById fails for one participant? Will it break the entire participant list update?
| role: ParticipantRole; | ||
| banned?: boolean; | ||
| } | ||
|
|
There was a problem hiding this comment.
What do you think about refactoring this to:
// (currently ParticipantDTO)
export interface ParticipantWithUserId {
userId: string;
connected: boolean;
ready: boolean;
raisedHand: boolean;
showHiddenColumns: boolean;
role: ParticipantRole;
banned?: boolean;
}
// currently Participant
export interface ParticipantWithUser {
user: Auth; // I also don't like the interface naming "Auth" but this isn't your problem.
connected: boolean;
ready: boolean;
raisedHand: boolean;
showHiddenColumns: boolean;
role: ParticipantRole;
banned?: boolean;
}
I think, this is easier to understand. What is you opinion about this @Schwehn42?
Description
Changing the frontend api requests that a separation of user and session in the backend is possible
Changelog
backendfrontendboard/thunk.js-> biggest problem was here since (user)sessions now only contains the userID but the frontend needed the full user object => added extra API request where neededKeep in mind that the frontend changes should not be kept like this. The goal is here to also refactor the frontend api in order to achieve the separation
Checklist
(Optional) Visual Changes